Skip to content

Add service provider factory support #18907

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Feb 22, 2020
Merged

Add service provider factory support #18907

merged 1 commit into from
Feb 22, 2020

Conversation

rynowak
Copy link
Member

@rynowak rynowak commented Feb 8, 2020

Fixes: #18814

This is an attempt to add back support on the Blazor WASM Host for using
ISerivceProviderFactory<>.

We previously had this support when the Blazor WASM host was a clone of
generic host, but I accidentally lost it when simplifying the host
(sorry :( ).

Summary of the changes (Less than 80 chars)

  • Detail 1
  • Detail 2

Addresses #bugnumber (in this specific format)

@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Feb 8, 2020
rynowak
rynowak commented Feb 8, 2020
View reviewed changes
@rynowak rynowak mentioned this pull request Feb 8, 2020
Closed
@rynowak rynowak force-pushed the rynowak/service-provider-factory branch from 214c536 to 4fa54fa Compare February 8, 2020 21:13
@ysmoradi
Copy link

ysmoradi commented Feb 9, 2020

When we can expect a release which contains this PR? Thanks in advance.

@rynowak
Copy link
Member Author

rynowak commented Feb 9, 2020

Next preview - probably about a month. We don't provide specific dates until it's ready.

@mkArtakMSFT mkArtakMSFT linked an issue Feb 9, 2020 that may be closed by this pull request
Allow using custom IServiceProvider in blazor 3.2 #18814
Closed
@davidfowl
Copy link
Member

davidfowl commented Feb 9, 2020 •
edited
Loading

@rynowak What happens when you have multiple Configure calls? Do I need to re-register the IServiceProviderFactory<T>?

@rynowak
Copy link
Member Author

rynowak commented Feb 10, 2020

What happens when you have multiple Configure calls? Do I need to re-register the IServiceProviderFactory?

Yes, but I'm not sure why you'd do that?

@davidfowl
Copy link
Member

Same reason you call configure services from multiple places though it’s less apparent that it’s useful here. The scenario would be another part of the application trying to call configure without overriding the factory

@rynowak
Copy link
Member Author

rynowak commented Feb 10, 2020 •
edited
Loading

Same reason you call configure services from multiple places though it’s less apparent that it’s useful here. The scenario would be another part of the application trying to call configure without overriding the factory

I'm not sure we're talking about the same thing - I might have answered you wrongly. This is the equivalent of ConfigureContainer. We have no equivalent of ConfigureServices, you just write code.

Are you saying that people abstract away multiple calls to ConfigureContainer today?

@davidfowl
Copy link
Member

Are you saying that people abstract away multiple calls to ConfigureContainer today?

I'm saying a reason to separate the registration of the factory itself and things that can configure the container is exactly that scenario. Less about abstracting the call to ConfigureContainer and more about calling it from different places in an application.

@rynowak
Copy link
Member Author

rynowak commented Feb 10, 2020

I'm saying a reason to separate the registration of the factory itself and things that can configure the container is exactly that scenario. Less about abstracting the call to ConfigureContainer and more about calling it from different places in an application.

Let's take this offline, I feel I'm having trouble understanding your feedback.

SteveSandersonMS
SteveSandersonMS approved these changes Feb 10, 2020
View reviewed changes
Copy link
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Let's close on the method name though.

@SteveSandersonMS
Copy link
Member

@rynowak Are we in a position to have unit tests for this too?

@rynowak
Copy link
Member Author

rynowak commented Feb 10, 2020

Are we in a position to have unit tests for this too?

No @davidfowl and I are still arguing 😆 - it has [Design] in the title.

javiercn
javiercn approved these changes Feb 21, 2020
View reviewed changes
Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with whatever you and @davidfowl agree for this.

@rynowak
Add service provider factory support
b1f3416 
Fixes: #18814

This adds back support on the Blazor WASM Host for using
ISerivceProviderFactory<>.

We previously had this support when the Blazor WASM host was a clone of
generic host, but I accidentally lost it when simplifying the host
(sorry :( ).
@rynowak rynowak force-pushed the rynowak/service-provider-factory branch from 4fa54fa to b1f3416 Compare February 21, 2020 23:47
@pranavkm pranavkm changed the title [Design] Add service provider factory support Add service provider factory support Feb 21, 2020
@pranavkm pranavkm added this to the blazor-wasm-3.2-preview2 milestone Feb 21, 2020
@rynowak rynowak merged commit 9c16db3 into blazor-wasm Feb 22, 2020
@rynowak rynowak deleted the rynowak/service-provider-factory branch February 22, 2020 00:28
@mkArtakMSFT mkArtakMSFT mentioned this pull request Mar 19, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SteveSandersonMS SteveSandersonMS SteveSandersonMS approved these changes

@javiercn javiercn javiercn approved these changes

@mkArtakMSFT mkArtakMSFT mkArtakMSFT approved these changes

@davidfowl davidfowl Awaiting requested review from davidfowl

Assignees
No one assigned
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Milestone
blazor-wasm-3.2-preview2
Development

Successfully merging this pull request may close these issues.

Allow using custom IServiceProvider in blazor 3.2
7 participants
@rynowak @ysmoradi @davidfowl @SteveSandersonMS @javiercn @mkArtakMSFT @pranavkm